Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Security Solutions] Removes one line non-null-assert using optional chaining and type narrowing #115127

Merged

Conversation

FrankHassanabad
Copy link
Contributor

@FrankHassanabad FrankHassanabad commented Oct 15, 2021

Summary

Removes one liner non-null-assert.

Instead of this line:

if (rule != null && spacesApi && outcome === 'conflict') {

We just check using the ? operator and type narrowing to remove the possibility of an error

if (rule?.alias_target_id != null && spacesApi && rule.outcome === 'conflict') {

The rule?.alias_target_id != null ensures that both rule and alias_target_id are not null/undefined

@FrankHassanabad FrankHassanabad self-assigned this Oct 15, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@FrankHassanabad FrankHassanabad changed the title Removes non-null-assert [Security Solutions] Removes one line non-null-assert Oct 15, 2021
@FrankHassanabad FrankHassanabad added auto-backport Deprecated - use backport:version if exact versions are needed v8.0.0 v7.16.0 Team:Security Solution Platform Security Solution Platform Team release_note:skip Skip the PR/issue when compiling release notes labels Oct 15, 2021
@FrankHassanabad FrankHassanabad marked this pull request as ready for review October 15, 2021 13:54
@FrankHassanabad FrankHassanabad requested a review from a team as a code owner October 15, 2021 13:54
@FrankHassanabad FrankHassanabad changed the title [Security Solutions] Removes one line non-null-assert [Security Solutions] Removes one line non-null-assert using optional chaining and type narrowing Oct 15, 2021
@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @FrankHassanabad

@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@FrankHassanabad
Copy link
Contributor Author

@elasticmachine merge upstream

@FrankHassanabad FrankHassanabad enabled auto-merge (squash) October 15, 2021 22:05
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 4.6MB 4.6MB -8.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @FrankHassanabad

@FrankHassanabad FrankHassanabad merged commit bf17898 into elastic:master Oct 16, 2021
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Oct 16, 2021
## Summary

Removes one liner non-null-assert.


Instead of this line:
```ts
if (rule != null && spacesApi && outcome === 'conflict') {
```

We just check using the `?` operator and type narrowing to remove the possibility of an error

```ts
if (rule?.alias_target_id != null && spacesApi && rule.outcome === 'conflict') {
```

The `rule?.alias_target_id != null` ensures that both `rule` and `alias_target_id` are not `null/undefined`
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

@FrankHassanabad FrankHassanabad deleted the one-line-remove-non-null-assert branch October 16, 2021 00:43
kibanamachine added a commit that referenced this pull request Oct 16, 2021
## Summary

Removes one liner non-null-assert.


Instead of this line:
```ts
if (rule != null && spacesApi && outcome === 'conflict') {
```

We just check using the `?` operator and type narrowing to remove the possibility of an error

```ts
if (rule?.alias_target_id != null && spacesApi && rule.outcome === 'conflict') {
```

The `rule?.alias_target_id != null` ensures that both `rule` and `alias_target_id` are not `null/undefined`

Co-authored-by: Frank Hassanabad <frank.hassanabad@elastic.co>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 18, 2021
…-migrate-away-from-injected-css-js

* 'master' of github.com:elastic/kibana: (237 commits)
  [Uptime] Added uptime query inspector panel (elastic#115170)
  [Osquery] Add packs (elastic#107345)
  [App Search] Allow for query parameter to indicate ingestion mechanism for new engines (elastic#115188)
  [Alerting] Active alerts do not recover after re-enabling a rule (elastic#111671)
  skip flaky tests.  elastic#115308, elastic#115313
  [Breaking] Remove deprecated `enabled` settings from plugins. (elastic#113495)
  skip flaky suite.  elastic#107057
  skip flaky tests. elastic#89052, elastic#113418, elastic#115304
  skip flaky test. elastic#113892
  Bump node to 16.11.1 (elastic#110684)
  [Security Solution] Restores Alerts table local storage persistence and the Remove Column action (elastic#114742)
  skip flaky suite.  elastic#115130
  one line remove assert (elastic#115127)
  Fixes migration bug where I was deleting attributes (elastic#115098)
  [Security Solutions] Fixes the newer notification system throttle resets and enabling immediate execution on first detection of a signal  (elastic#114214)
  [build] Dockerfile update (elastic#115237)
  Fixes Cypress flake cypress test (elastic#115270)
  Disable APM e2e tests
  log an invalid type for SO (elastic#115175)
  [Fleet] Don't auto upgrade policies for AUTO_UPDATE packages (elastic#115199)
  ...

# Conflicts:
#	src/plugins/dashboard/public/application/dashboard_app.tsx
#	src/plugins/dashboard/public/types.ts
#	x-pack/plugins/reporting/server/lib/layouts/print_layout.ts
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 18, 2021
…-link-to-kibana-app

* 'master' of github.com:elastic/kibana: (287 commits)
  [Security Solution][Endpoint] Change `trustedAppByPolicyEnabled` flag to `true` by default (elastic#115264)
  [APM] generator: support error events and application metrics (elastic#115311)
  [kibanaUtils] Don't import full `semver` client side (elastic#114986)
  [RAC] Link inventory alerts to the right inventory view (elastic#113553)
  [Uptime] Added uptime query inspector panel (elastic#115170)
  [Osquery] Add packs (elastic#107345)
  [App Search] Allow for query parameter to indicate ingestion mechanism for new engines (elastic#115188)
  [Alerting] Active alerts do not recover after re-enabling a rule (elastic#111671)
  skip flaky tests.  elastic#115308, elastic#115313
  [Breaking] Remove deprecated `enabled` settings from plugins. (elastic#113495)
  skip flaky suite.  elastic#107057
  skip flaky tests. elastic#89052, elastic#113418, elastic#115304
  skip flaky test. elastic#113892
  Bump node to 16.11.1 (elastic#110684)
  [Security Solution] Restores Alerts table local storage persistence and the Remove Column action (elastic#114742)
  skip flaky suite.  elastic#115130
  one line remove assert (elastic#115127)
  Fixes migration bug where I was deleting attributes (elastic#115098)
  [Security Solutions] Fixes the newer notification system throttle resets and enabling immediate execution on first detection of a signal  (elastic#114214)
  [build] Dockerfile update (elastic#115237)
  ...

# Conflicts:
#	x-pack/plugins/reporting/public/management/__snapshots__/report_listing.test.tsx.snap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes Team:Security Solution Platform Security Solution Platform Team v7.16.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants